-
-
Notifications
You must be signed in to change notification settings - Fork 2.3k
Remove the ability to skip device verification during login #31041
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: develop
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM otherwise.
| export async function doesServerSupportCrossSigning(cli: MatrixClient): Promise<boolean> { | ||
| // cross-signing support was added to Matrix in MSC1756, which landed in spec v1.1 | ||
| return ( | ||
| (await cli.isVersionSupported("v1.1")) || | ||
| (await cli.doesServerSupportUnstableFeature("org.matrix.e2e_cross_signing")) | ||
| ); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd suggest dropping this check altogether. I'm not sure about EW, but the js-sdk claims a minimum supported server version of 1.1, so we should be good.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That sounds sane, if js-sdk flags that your server version is not supported then you will not be able to login anyway iirc
My understanding is that this does not fix that issue, because this PR does nothing to change the behaviour for existing, logged-in, but unverified devices, whereas #28464 encompasses things like reminding people to verify their devices, and, ultimately, locking out the whole UI until they have verified? |
|
Marking as blocked as per the warning triangle in the description |
Right, I was going by the "Context" section of that issue's description, which only talks about logging in. But looking at it again, it does indeed look like that issue is a parent issue that encompasses other work. I'll create a new issue for this. |
matrix-js-sdk already requires spec version 1.1 support
13513e6 to
d61c28d
Compare
Fixes #31073
Since we will, at some point, no longer send megolm sessions to unverified devices, we will force users on login to verify their devices to prevent them from getting UTDs.
Checklist
public/exportedsymbols have accurate TSDoc documentation.